Skip to content

[FIXED] KeyValue not found after server restarts#5054

Merged
derekcollison merged 2 commits into
mainfrom
kv-not-found
Feb 9, 2024
Merged

[FIXED] KeyValue not found after server restarts#5054
derekcollison merged 2 commits into
mainfrom
kv-not-found

Conversation

@derekcollison
Copy link
Copy Markdown
Member

Do not force sequence mismatches and possible dataloss on first sequence move past a catchup request.

When a server restarts and does a stream catchup where the snapshot from the leader is for messages it no longer has, we would create a sequence mismatch reset on the follower.

Also on a catchup if the stream state has moved past the updated request range from a follower, meaning first for the new state is past the last of the request, extend and continue catchup.

Signed-off-by: Derek Collison derek@nats.io

…nce move past a catchup request.

When a server restarts and does a stream catchup where the snapshot from the leader is for messages it no longer has, we would create a sequence mismatch reset on the follower.
Also on a catchup if the stream state has moved past the updated request range from a follower, meaning first for the new state is past the last of the request, extend and continue catchup.

Signed-off-by: Derek Collison <derek@nats.io>
Signed-off-by: Derek Collison <derek@nats.io>
@derekcollison derekcollison requested a review from a team as a code owner February 9, 2024 20:01
Copy link
Copy Markdown
Member

@wallyqs wallyqs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@derekcollison derekcollison merged commit eadab32 into main Feb 9, 2024
@derekcollison derekcollison deleted the kv-not-found branch February 9, 2024 23:37
derekcollison added a commit that referenced this pull request Feb 10, 2024
Do not force sequence mismatches and possible dataloss on first sequence
move past a catchup request.

When a server restarts and does a stream catchup where the snapshot from
the leader is for messages it no longer has, we would create a sequence
mismatch reset on the follower.
    
Also on a catchup if the stream state has moved past the updated request
range from a follower, meaning first for the new state is past the last
of the request, extend and continue catchup.

Signed-off-by: Derek Collison <derek@nats.io>

---------

Signed-off-by: Derek Collison <derek@nats.io>
wallyqs pushed a commit that referenced this pull request Feb 13, 2024
Do not force sequence mismatches and possible dataloss on first sequence
move past a catchup request.

When a server restarts and does a stream catchup where the snapshot from
the leader is for messages it no longer has, we would create a sequence
mismatch reset on the follower.
    
Also on a catchup if the stream state has moved past the updated request
range from a follower, meaning first for the new state is past the last
of the request, extend and continue catchup.

Signed-off-by: Derek Collison <derek@nats.io>

---------

Signed-off-by: Derek Collison <derek@nats.io>
wallyqs added a commit that referenced this pull request Feb 14, 2024
derekcollison added a commit that referenced this pull request Oct 24, 2024
When we're being asked to provide data within a range during catchup, we
should not extend that range and provide more data. Especially since
that range was defined by a snapshot, which also specifies which RAFT
entries should be sent after processing that snapshot. This would just
result in duplicated work and possible desync for the follower, so these
lines can safely be removed.

**Timeline of these lines:**
Previously when receiving a catchup request the `FirstSeq` could be
moved up to match what the state says:
[[IMPROVED] Catchup improvements
#3348](https://github.com/nats-io/nats-server/pull/3348/files#diff-5cb252c37caef12e7027803018861c82724b120ddb62cfedc2f36addf57f6970R7132-R7138)
(August, 2022)

Afterward this was removed in favor of only extending the `LastSeq`:
[[FIXED] KeyValue not found after server restarts
#5054](https://github.com/nats-io/nats-server/pull/5054/files#diff-5cb252c37caef12e7027803018861c82724b120ddb62cfedc2f36addf57f6970R8579-R8588)
(February, 2024)

This was done to solve for KeyValue not found issues.
However this change would have also fixed that case:
[[FIXED] Do not bump clfs on seq mismatch when before stream LastSeq
#5821](https://github.com/nats-io/nats-server/pull/5821/files#diff-2f4991438bb868a8587303cde9107f83127e88ad70bd19d5c6a31c238a20c299R4694-R4699)
(August, 2024)

Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants